Skip to content

Conversation

@deborahgu
Copy link
Member

  • pulls in any extra sources configured for credentials and edxapp
  • technically I suppose there could be problems if they aren't in the environment but we are absolutely populating the environment in edx-internal
  • added to the credentials docker file twice, because there's 2 different potential images where we build translations

- pulls in any extra sources configured for credentials and edxapp
-  technically I suppose there could be problems if they aren't in the environment but we are absolutely populating the environment in edx-internal
-  added to the  credentials docker file twice, because there's 2 different potential images where we build translations
Copilot AI review requested due to automatic review settings January 29, 2026 21:47

This comment was marked as spam.

ARG OPENEDX_ATLAS_EXTRA_SOURCES
ARG OPENEDX_TRANSLATIONS_VERSION
ARG OPENEDX_TRANSLATIONS_REPO
ENV ATLAS_EXTRA_SOURCES="--repository=$OPENEDX_ATLAS_EXTRA_SOURCES"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should work, but it might be cleaner to include the env var as an export line in the translations RUN block. (Less broad impact, and no inclusion in final image's runtime environment.)

Copy link
Member

@timmc-edx timmc-edx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added suggestion for a more precise application of env vars, but approving.

mild re-factoring of environment variable per  code review suggestion
@deborahgu deborahgu merged commit 9054f48 into main Feb 3, 2026
3 checks passed
@deborahgu deborahgu deleted the adding-credentials-themes-to-build-targets branch February 3, 2026 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants